Skip to content

Guard the parity pin header against declaration-order breaks (#558) - #594

Merged
localai-bot merged 2 commits into
mainfrom
row/FIX-HF-SNAPSHOT-ORDER-551
Aug 13, 2026
Merged

Guard the parity pin header against declaration-order breaks (#558)#594
localai-bot merged 2 commits into
mainfrom
row/FIX-HF-SNAPSHOT-ORDER-551

Conversation

@localai-bot

Copy link
Copy Markdown
Collaborator

Closes the guard half of the now-closed #551/#546. The header defect itself is already fixed on main by fafa16f0f; this branch was re-scoped onto that fix and its own header hunk dropped, so tests/parity/hf_snapshot.h is untouched here (git diff origin/main -- tests/parity/hf_snapshot.h is empty).

Issue: #558

What broke, and why the build did not catch it

af8170154 added Nemotron35LightningSnapshot() calling HfSnapshot eleven lines before its declaration. 14 TUs that include the header failed to compile. Because those suites are checkpoint-gated, ctest reported them as ***Not Run — which reads as missing checkpoints, not a build break.

A full C++ build does catch this. The commit that broke it was records-and-evidence work that never built C++ at all, and agent-preflight.sh does not build. That lane is what the guard covers.

The guard is not redundant — measured

With af8170154's broken header in place and this method deleted, the entire preflight Python lane is green: all 41 checkers and 40 mutation suites ok. The break is seen by this method alone.

It is a real compiler invocation, not a pattern match, so it generalizes. Review mutations against main's fixed header:

mutation result
original ordering break re-applied FAIL, quotes 'HfSnapshot' was not declared in this scope
duplicate definition (the merge hazard below) FAIL, quotes redefinition of ...Nemotron35LightningSnapshot()
revision-constant name typo FAIL
dropped #endif FAIL, unterminated #ifndef
dropped #include <cstdlib> correctly PASSES — still satisfied transitively

A merge hazard git cannot see

Two independent repairs of the same defect, relocating the same block to different places, auto-merge with no conflict into a header defining Nemotron35LightningSnapshot() twice, re-breaking the same 14 TUs:

Auto-merging tests/parity/hf_snapshot.h
Automatic merge went well
$ grep -c "inline std::string Nemotron35LightningSnapshot" tests/parity/hf_snapshot.h
2

That is precisely how this branch would have re-broken main had it landed unrescoped. The guard catches it.

Two review findings closed

  • The skip was invisible. skipTest exits 0 and agent-preflight.sh prints green while swallowing stdout, so a compiler-less box silently no-opped the guard. It now FAILS when CI is set, and still skips otherwise. Both branches proven.
  • $CXX was truncated to one token, so ccache g++ / env g++ gave a spurious RED. Fixed. The fallback is deliberately bounded: only exit 126/127 falls through (the launcher could not exec the compiler); a compiler that ran and rejected the header — exit 1 or 4 — is reported RED and never masked. Verified in both directions.

Scope and gate

Two files, +117/−0. No src/, no include/, no C++ tests, no goldens, no checker weakened, no assertion deleted, inventory counts unchanged (scripts/check-snapshot-pins.py itself is byte-identical).

Merged tree at ea43379ce: test_check_snapshot_pins 20/20 OK, check-snapshot-pins OK + SELF-TEST OK, check-agent-record OK, agent-preflight.sh --staged green. Main's current header compiles standalone, so the guard passes against it.

Known limitations, recorded not hidden

Hardcodes -std=c++20 (matches CMakeLists.txt:35 today), carries no -Wall -Wextra -Werror, and does not prove include self-containment. It guards this one header — a new header under tests/parity/ inherits nothing; the honest generalization is a per-header sweep, noted in #558 alongside #408.

🤖 Generated with Claude Code

mudler added 2 commits August 13, 2026 00:42
…header main already fixed (#558)

FOLLOWING_AGENTS_PROTOCOL

`main` fixed the declaration-order break itself at `fafa16f0f` (PR #556) while
this branch carried its own repair, and #551/#546 are CLOSED. Two independent
moves of one block do not conflict: merging the two repairs auto-merged clean
into a header that DEFINED `Nemotron35LightningSnapshot()` twice and re-broke
the same 14 TUs. So this branch was RESET onto `fafa16f0f` and re-applies only
the part that is still owed -- the guard, now tracked by #558.
`git diff origin/main -- tests/parity/hf_snapshot.h` is EMPTY: main's header
survives byte-for-byte (md5 048c84eb5cc20146f4be45f7747b8a34).

WHY THE GUARD IS STILL WANTED. Nothing else in the Python lane sees the defect
-- delete this method and `scripts/check-snapshot-pins.py` exits 0 on a header
no TU can include. A full C++ build does catch it, and that is exactly the
point: the commit that broke it was records and evidence and never built C++ at
all, and all 14 TUs that include the header are checkpoint-gated, so a serial
`ctest` reported `***Not Run`, which on a box with no snapshots reads as a
missing checkpoint rather than a build break. This suite already runs from
`scripts/agent-preflight.sh` and from CI's record lane (`ci.yml:172`), needs no
CMake, no build tree, no GPU and no checkpoint, and costs ~0.3s.

TWO REVIEW FINDINGS CLOSED WITH IT.

1. A SKIP WAS INVISIBLE. `skipTest` exits 0 and `agent-preflight.sh`'s `run()`
   prints a green `ok` while swallowing stdout, so a compiler-less box turned
   the guard into a silent no-op. Measured before:
   `env -u CXX CI=1 PATH=/nonexistent python3 -m unittest ...` -> `OK
   (skipped=1)`, exit 0. It now FAILS when `CI` is set and skips otherwise:
   with `CI=1` and no compiler, `AssertionError: CI must be able to
   syntax-check tests/parity/hf_snapshot.h ... A skip here exits 0 and reads as
   a pass`, exit 1; without `CI`, `OK (skipped=1)`, exit 0.

2. `$CXX` LOST ITS LAUNCHER. `configured[:1]` kept only the first token, so
   `ccache g++` / `sccache clang++` / `distcc g++` ran the LAUNCHER with the
   compiler's flags. Measured before, with `CXX="env g++"`:
   `AssertionError: 0 != 125 ... /usr/bin/env: invalid option -- 's'` -- a red
   naming this header for a defect in the environment. The whole token list is
   now the argv prefix; the same `CXX="env g++"` is green. A candidate that
   cannot be LAUNCHED (126/127) falls through to the next; every other status
   is a compiler's verdict and is reported, so `CXX` exiting 1 is still RED and
   is not masked by the fallback.

NEGATIVE MUTATION, against main's fixed header, in a scratch tree (the repo
copy was never touched -- md5 unchanged before and after):
  * M1, the #551 ordering restored: FAILED, `'HfSnapshot' was not declared in
    this scope`, quoted verbatim by the assertion message.
  * M2, the auto-merge hazard above (the block defined twice): FAILED,
    `redefinition of 'std::string parity::Nemotron35LightningSnapshot()'`.
    No other check in the repo sees that one.

The intake row is #558 and not #551: the table is for OPEN issues, #551 closed
at 2026-08-12T23:53:23Z, and the row `f2fe496f` added went with the reset.
`Row` is `—` -- the guard owns no capability, so no matrix row and no count
that `check-agent-record.py` enforces changes. Placement is sorted, not
appended: within the `—` entries the file orders by issue number ascending and
558 is the highest, so its sort position is the last row. The anchor was
asserted unique (count == 1) before the insert; the result is 1 insertion, 0
deletions.

Gates on the final tree: clean out-of-tree `-Werror` Release build on local
disk (`build-gate/`, CPU: no CUDA toolkit on this box) exit 0; SERIAL `ctest`
exit 0; `scripts/agent-preflight.sh` exit 0 before the edits and `--staged`
before this commit. The 13 checkpoint-gated parity suites emit their loud
checkpoint-absent SKIP with 0 assertions and are NOT presented as coverage; no
GPU gate and no checkpoint gate exists on this box.

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
FOLLOWING_AGENTS_PROTOCOL

Bring the guard branch current before opening its PR. No conflict; the roadmap
issue table auto-merged to exactly 1 added line with no other key disturbed.

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
@localai-bot
localai-bot merged commit 7572b0f into main Aug 13, 2026
0 of 44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

main is RED: hf_snapshot.h calls HfSnapshot before it is declared, blocking 14 parity TUs including the SACRED gates

2 participants